Skip to content

fix: use file-local createRequire for relative lazy requires in src/#1008

Merged
BYK merged 2 commits into
mainfrom
fix/require-shim-relative-paths
May 22, 2026
Merged

fix: use file-local createRequire for relative lazy requires in src/#1008
BYK merged 2 commits into
mainfrom
fix/require-shim-relative-paths

Conversation

@betegon
Copy link
Copy Markdown
Member

@betegon betegon commented May 22, 2026

Problem

After the Bun→Node migration, pnpm cli and any direct pnpm tsx src/bin.ts invocation were broken:

Fatal: Error: Cannot find module '../telemetry.js'
Require stack:
- /Users/bete/code/init/cli/package.json

Every single invocation crashed before executing any command.

Root cause

require-shim.mjs installs a global require() anchored at package.json (project root) so that node:* builtins work in ESM. This has two failure modes:

  1. Relative paths resolve wrongrequire("../telemetry.js") from src/lib/db/ resolves relative to the project root, not the calling file: /project-root/../telemetry.js → does not exist
  2. Shim not loaded at all — when invoking via pnpm tsx /path/to/bin.ts from outside the cli/ directory, the --import ./script/require-shim.mjs in the pnpm tsx script is never applied, so require doesn't exist at all in ESM scope

The shim comment said relative require() calls in src/ "don't execute during tsx script runs." That was true when pnpm tsx was only used for script/*.ts utilities — but pnpm cli also uses tsx for src/bin.ts, which hits all these paths on every invocation.

What was broken

A full audit found 13 bare require() calls across 9 files — two crashed hard, the rest failed silently:

File Symptom
src/lib/db/sqlite.ts Fatal crash — module-level require, fires on import
src/lib/db/index.ts Fatal crashrequire("../telemetry.js") wrong path
src/lib/list-command.ts Silent — subcommand interception (sentry issue view tip) never worked
src/lib/telemetry.ts Silent — DB path fell back to hardcoded ~/.sentry/cli.db; Sentry reporter never attached
src/lib/db/schema.ts Silent — schema auto-repair never ran
src/lib/db/migration.ts Silent — JSON→SQLite migration silently skipped
src/lib/custom-ca.ts Silent — setDefaultCACertificates always undefined on Node 24+
src/lib/logger.ts Silent — Sentry consola reporter never attached
src/lib/init/ui/ink-ui.ts Silent — SEA binary detection always false

Fix

Replace every bare require() in src/ with a file-local createRequire(import.meta.url). This resolves paths relative to the calling file regardless of how the CLI was invoked or where the shim is anchored. Also corrects the shim comment.

Also removed 3 dead skipIf(!isBunSqlite) tests in telemetry.test.ts that were permanently skipped on Node, and updated two stale Bun.* references in comments.

Test plan

# Before (from any directory)
pnpm tsx /path/to/cli/src/bin.ts init
→ Fatal: ReferenceError: require is not defined

# Before (from cli/ dir)
pnpm cli issue view
→ Fatal: Cannot find module '../telemetry.js'

# After (both invocations)
→ Expected argument for issue  (DB init works, routing works)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-1008/

Built to branch gh-pages at 2026-05-22 20:07 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

Codecov Results 📊

✅ Patch coverage is 100.00%. Project has 4235 uncovered lines.
✅ Project coverage is 81.87%. Comparing base (base) to head (head).

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    81.86%    81.87%    +0.01%
==========================================
  Files          328       328         —
  Lines        23351     23359        +8
  Branches     15114     15114         —
==========================================
+ Hits         19115     19124        +9
- Misses        4236      4235        -1
- Partials      1621      1621         —

Generated by Codecov Action

@betegon betegon force-pushed the fix/require-shim-relative-paths branch 2 times, most recently from 1df193c to e75f2ba Compare May 22, 2026 17:09
The require-shim.mjs anchors globalThis.require at the project root
(package.json), which works for node:* builtins but breaks relative
paths — require("../telemetry.js") resolves to /project-root/../
instead of relative to the calling file.

The shim assumed relative require() calls in src/ only ran during
script/*.ts tsx runs, but pnpm cli (which also uses tsx) hits both
db/index.ts and list-command.ts at runtime. db/index.ts crashed hard;
list-command.ts was silently swallowed by try/catch, leaving
getSubcommandsForRoute() always returning an empty map.

Fix: replace the global require() with a file-local createRequire
anchored to import.meta.url in each affected file. Update the shim
comment to reflect the constraint.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@betegon betegon force-pushed the fix/require-shim-relative-paths branch from e75f2ba to 95897c6 Compare May 22, 2026 17:17
@betegon betegon marked this pull request as ready for review May 22, 2026 17:49
@betegon betegon requested a review from BYK May 22, 2026 17:58
…ile import hook

- Keep _require (createRequire) for node builtins and npm packages
- Use bare require() for relative paths (../telemetry.js, ./index.js, etc.)
  so esbuild resolves them at bundle time
- Add registerHooks loader for with { type: 'file' } in tsx dev mode
Comment thread src/lib/db/index.ts
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 01b8d18. Configure here.

Comment thread script/build.ts
Comment thread src/lib/db/schema.ts
@BYK BYK merged commit d3c0966 into main May 22, 2026
29 checks passed
@BYK BYK deleted the fix/require-shim-relative-paths branch May 22, 2026 20:17
BYK added a commit that referenced this pull request May 22, 2026
Documents the critical esbuild bundling rule that caused two
post-migration bugs (#1008, #1009):

**esbuild only statically resolves bare `require()` calls.** Any aliased
require (`_require`, `localRequire`, etc.) passes through as-is into the
bundle and breaks at runtime when used with relative paths.

Adds a reference table and 4 key rules to AGENTS.md so AI agents and
contributors avoid this pattern.

Filed #1010 for the companion testing improvements (deeper smoke tests
for binary + npm bundle).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants